Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dynamic regional dex rating #3900

Merged
merged 3 commits into from
Jan 5, 2024
Merged

Conversation

Bassoonian
Copy link
Collaborator

This PR completely overhauls the regional dex rating. Features:

  • There no longer is a hardcoded limit of 200 in the rating. Instead, it dynamically adapts to the amount of species in the regional dex and the amount of quotes Birch has, evenly distributing them across the size of the regional dex.
  • The two different functions in Match Call and actually talking to Birch have been condensed into a single function.
  • Rather than hardcoding Jirachi and Deoxys (and how they affect what counts as a complete dex), there is now a static array that consists of species that are ignored for regional dex rating purposes. They are taken into account for the formulas mentioned above.
  • This also saves a whopping 496 bytes!

This is currently based on master since it's a bugfix (imo), but can be rebased to upcoming if considered too invasive for master.

Issue(s) that this PR fixes

Fixes #3874

Discord contact info

bassoonian

@pkmnsnfrn
Copy link
Collaborator

should not be blocking: I understand that the static array for Jirachi and Deoxys work just fine, but that sounds like it should be a member of the Pokemon struct, like doesNotCountForRegionalPokedex. This concepts exists in every main series game from Gen 3 onwards and editing that property would likely be on every developer's list for their game's regional dex.

@Bassoonian
Copy link
Collaborator Author

should not be blocking: I understand that the static array for Jirachi and Deoxys work just fine, but that sounds like it should be a member of the Pokemon struct, like doesNotCountForRegionalPokedex. This concepts exists in every main series game from Gen 3 onwards and editing that property would likely be on every developer's list for their game's regional dex.

I believe making it a member of the struct will make the process slightly longer since we need to check that value for each mon in the dex as opposed to just going through the list, but it's a very fair remark. Since this is going to master, I think we could potentially consider it blocking, so feedback from others would be highly appreciated here

@pkmnsnfrn
Copy link
Collaborator

imho, merge this one as is, make an issue for later to address doesNotCountForRegionalPokedex. You fixed the bug, this is a bug fix, we can optimize later

@AlexOn1ine
Copy link
Collaborator

AlexOn1ine commented Jan 5, 2024

Can anybody provide a save that this can be tested with?

/Edi sorry, I'm stupid. I don't need a save.

Copy link
Collaborator

@AlexOn1ine AlexOn1ine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's fine if this goes to master since it fixes an obvious bug.

@pkmnsnfrn Could you please open an issue ticket?

@AlexOn1ine AlexOn1ine merged commit f9c21af into rh-hideout:master Jan 5, 2024
1 check passed
@Bassoonian Bassoonian deleted the dexcountfix branch January 21, 2024 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prof. Birch cannot identify Pokedex correctly
3 participants